Skip to content

Fix(Tracer): display of values created by builtins list, is_... does not show anything #3259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jul 28, 2025
Merged

Fix(Tracer): display of values created by builtins list, is_... does not show anything #3259

merged 4 commits into from
Jul 28, 2025

Conversation

@CATISNOTSODIUM
Copy link
Member

@CATISNOTSODIUM CATISNOTSODIUM commented Jul 26, 2025 •
edited
Loading

Description

Refer to this source-academy/js-slang#1797 for more details.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

Try running predeclared functions that output non-number values and test whether the values are properly rendered or not.

is_null(3);
is_pair(10);
pair(is_null(false), is_null(true));
list(is_null(false), is_boolean(3), is_function(4), is_undefined(5), is_undefined(undefined));

Here is the example output.
image

Checklist

  • I have tested this code

@CATISNOTSODIUM CATISNOTSODIUM self-assigned this Jul 26, 2025
@CATISNOTSODIUM CATISNOTSODIUM added Bug Something isn't working critical Fixing this is mission-critical labels Jul 26, 2025
@coveralls
Copy link

coveralls commented Jul 26, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 16574599023

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 5 (0.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.005%) to 44.086%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commons/sideContent/content/SideContentSubstVisualizer.tsx 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
src/commons/sideContent/content/SideContentSubstVisualizer.tsx 1 31.58%
Totals Coverage Status
Change from base Build 16574592230: -0.005%
Covered Lines: 20347
Relevant Lines: 48260
💛 - Coveralls

CATISNOTSODIUM added 2 commits July 29, 2025 00:19
@RichDom2185
fix(tracer): rendering issue with literal AST
29ca5a0
@RichDom2185
chore: run linter
2059372 
# Conflicts:
#	src/commons/sideContent/content/SideContentSubstVisualizer.tsx
RichDom2185
RichDom2185 approved these changes Jul 28, 2025
View reviewed changes
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@RichDom2185 RichDom2185 enabled auto-merge (squash) July 28, 2025 16:27
@RichDom2185 RichDom2185 merged commit a80c95c into master Jul 28, 2025
9 checks passed
@RichDom2185 RichDom2185 deleted the stepper/rewrite branch July 28, 2025 16:30
@RichDom2185 RichDom2185 mentioned this pull request Jul 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RichDom2185 RichDom2185 RichDom2185 approved these changes

Assignees

@CATISNOTSODIUM CATISNOTSODIUM

Labels

Bug Something isn't working critical Fixing this is mission-critical

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@CATISNOTSODIUM @coveralls @RichDom2185